Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CMake] Updated CMake script to change layout of build files. #1224

Merged
merged 1 commit into from Mar 12, 2020

Conversation

AYESDIE
Copy link
Member

@AYESDIE AYESDIE commented Mar 11, 2020

Fixes on #1131.

Changes:

  • Updated CMakeLists.txt. Inside the build folder, it makes bin and lib (ETL, include). Execution still require these commands.
  • Updated .gitignore.

@ice0
Copy link
Collaborator

ice0 commented Mar 11, 2020

Hi, @AYESDIE!

This can be done with 3 lines of code.
Please see here: https://cmake.org/cmake/help/v3.0/manual/cmake-variables.7.html

CMAKE_RUNTIME_OUTPUT_DIRECTORY
CMAKE_LIBRARY_OUTPUT_DIRECTORY
CMAKE_ARCHIVE_OUTPUT_DIRECTORY

also synfig-core/src/modules/CMakeLists.txt modules must goes to lib/synfig/modules/

@ice0 ice0 self-requested a review March 11, 2020 17:14
@AYESDIE
Copy link
Member Author

AYESDIE commented Mar 11, 2020

@ice0 Ah, rather than going for a per-target basis, updating those directory in the root CMakeLists.txt seems better. I'll work on the changes and push it soon.

Should I make changes on how ETL and include is built?

@ice0
Copy link
Collaborator

ice0 commented Mar 11, 2020

updating those directory in the root CMakeLists.txt seems better

Yes. Also this will be much easier to support in the future.

Should I make changes on how ETL and include is built?

I did not think about them. Therefore, I suggest just skipping them for now and back to them later.

@AYESDIE
Copy link
Member Author

AYESDIE commented Mar 11, 2020

Yes. Also this will be much easier to support in the future.

Agreed :)

I force pushed the new changes. I removed the ETL and include build part as of now.

CMakeLists.txt Outdated Show resolved Hide resolved
synfig-core/src/synfig/CMakeLists.txt Outdated Show resolved Hide resolved
synfig-studio/src/synfigapp/CMakeLists.txt Outdated Show resolved Hide resolved
.gitignore Show resolved Hide resolved
@ice0
Copy link
Collaborator

ice0 commented Mar 11, 2020

@AYESDIE, also i want to ask, which OS you are using?

@AYESDIE
Copy link
Member Author

AYESDIE commented Mar 11, 2020

I am on Ubuntu 19.10.

@ice0
Copy link
Collaborator

ice0 commented Mar 11, 2020

I am on Ubuntu 19.10.

Awesome!

Also forgot to mention, please change the path to ${CMAKE_BINARY_DIR}/output/->lib/bin/etc

Copy link
Collaborator

@ice0 ice0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also please squash commits :)

CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
@AYESDIE
Copy link
Member Author

AYESDIE commented Mar 11, 2020

I was building on my end to make sure things were in correct places :))

@ice0
Copy link
Collaborator

ice0 commented Mar 11, 2020

Ok, also please check make install target, and you see why i add new comments :)

P.S. use cmake .. -DCMAKE_INSTALL_PREFIX=your_custom_install_dir

@AYESDIE
Copy link
Member Author

AYESDIE commented Mar 11, 2020

Ok, also please check make install target, and you see why i add new comments :)

It worked the way it was before too, since I kept on checking it with make install on each commit, so I thought it was correct. But I realize I overcomplicated things abit.

@ice0
Copy link
Collaborator

ice0 commented Mar 11, 2020

Awesome work! This is exactly what I wanted!

Therefore, I propose to continue tomorrow.

P.S. I will merge your changes after all the checks have been completed.

@AYESDIE
Copy link
Member Author

AYESDIE commented Mar 11, 2020

Thank you for guiding :)

The next thing will be to refactor the build_images target, right? #1132 to be exact?

@ice0
Copy link
Collaborator

ice0 commented Mar 11, 2020

Yes. But i want to add one more simple issue for tomorrow.
We need to add /etc/modules.cfg and /share/synfig/*.glade files to do synfigstudio runnable. (#1225)

In this way, new students will be able to use IDEs such as CLion or NetBeans to build Synfig Studio and check they changes.

@AYESDIE
Copy link
Member Author

AYESDIE commented Mar 11, 2020

Ah, sounds good to me. I'll take a look at it tomorrow :)

CMakeLists.txt Outdated Show resolved Hide resolved
@@ -43,6 +43,9 @@ set(MODS_ENABLED
# mptr_mplayer # - "This code has vulnerabilities"
)

set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/output/lib/modules)
set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/output/lib/modules)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just checked. This path must be lib/synfig/modules not lib/modules.

@ice0 ice0 modified the milestone: GSoC 2020: CMake Mar 12, 2020
@AYESDIE AYESDIE changed the title Updated CMake script to change layout of build files. [CMake] Updated CMake script to change layout of build files. Mar 12, 2020
@ice0 ice0 merged commit 50bab45 into synfig:master Mar 12, 2020
@ice0
Copy link
Collaborator

ice0 commented Mar 12, 2020

Merged! Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants